[JTS] Use common branch weight downscaling #153738
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
@llvm/pr-subscribers-llvm-transforms Author: Mircea Trofin (mtrofin) ChangesThis also fixes a bug introduced accidentally in #153651, whereby the Full diff: https://github.com/llvm/llvm-project/pull/153738.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Scalar/JumpTableToSwitch.cpp b/llvm/lib/Transforms/Scalar/JumpTableToSwitch.cpp
index 9dde131185cf8..9d915d0d967e5 100644
--- a/llvm/lib/Transforms/Scalar/JumpTableToSwitch.cpp
+++ b/llvm/lib/Transforms/Scalar/JumpTableToSwitch.cpp
@@ -159,6 +159,10 @@ expandToSwitch(CallBase *CB, const JumpTableTy &JT, DomTreeUpdater &DTU,
DTUpdates.push_back({DominatorTree::Insert, B, Tail});
CallBase *Call = cast<CallBase>(CB->clone());
+ // The MD_prof metadata (VP kind), if it existed, can be dropped, it doesn't
+ // make sense on a direct call. Note that the values are used for the branch
+ // weights of the switch.
+ Call->setMetadata(LLVMContext::MD_prof, nullptr);
Call->setCalledFunction(Func);
Call->insertInto(B, B->end());
Switch->addCase(
@@ -180,14 +184,8 @@ expandToSwitch(CallBase *CB, const JumpTableTy &JT, DomTreeUpdater &DTU,
if (HadProfile && !ProfcheckDisableMetadataFixes) {
// At least one of the targets must've been taken.
assert(llvm::any_of(BranchWeights, [](uint64_t V) { return V != 0; }));
- // FIXME: this duplicates logic in instrumentation. Note: since there's at
- // least a nonzero and these are unsigned values, it follows MaxBW != 0.
- uint64_t MaxBW = *llvm::max_element(BranchWeights);
- SmallVector<uint32_t> ScaledBranchWeights(
- llvm::map_range(BranchWeights, [MaxBW](uint64_t V) {
- return static_cast<uint32_t>(V / MaxBW);
- }));
- setBranchWeights(*Switch, ScaledBranchWeights, /*IsExpected=*/false);
+ setBranchWeights(*Switch, downscaleWeights(BranchWeights),
+ /*IsExpected=*/false);
} else
setExplicitlyUnknownBranchWeights(*Switch);
if (PHI)
diff --git a/llvm/test/Transforms/JumpTableToSwitch/basic.ll b/llvm/test/Transforms/JumpTableToSwitch/basic.ll
index 577c2adaf5afa..e3c032b9d4bf7 100644
--- a/llvm/test/Transforms/JumpTableToSwitch/basic.ll
+++ b/llvm/test/Transforms/JumpTableToSwitch/basic.ll
@@ -1,14 +1,39 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals all --version 4
; RUN: opt < %s -passes=jump-table-to-switch -verify-dom-info -S | FileCheck %s
; RUN: opt < %s -passes=jump-table-to-switch -jump-table-to-switch-size-threshold=0 -verify-dom-info -S | FileCheck %s --check-prefix=THRESHOLD-0
@func_array = constant [2 x ptr] [ptr @func0, ptr @func1]
+;.
+; CHECK: @func_array = constant [2 x ptr] [ptr @func0, ptr @func1]
+; CHECK: @void_func_array = constant [2 x ptr] [ptr @void_func0, ptr @void_func1]
+; CHECK: @func_array_addrspace_42 = addrspace(42) constant [2 x ptr addrspace(42)] [ptr addrspace(42) @func0_addrspace_42, ptr addrspace(42) @func1_addrspace_42]
+;.
+; THRESHOLD-0: @func_array = constant [2 x ptr] [ptr @func0, ptr @func1]
+; THRESHOLD-0: @void_func_array = constant [2 x ptr] [ptr @void_func0, ptr @void_func1]
+; THRESHOLD-0: @func_array_addrspace_42 = addrspace(42) constant [2 x ptr addrspace(42)] [ptr addrspace(42) @func0_addrspace_42, ptr addrspace(42) @func1_addrspace_42]
+;.
define i32 @func0() !guid !0 {
+; CHECK-LABEL: define i32 @func0(
+; CHECK-SAME: ) !guid [[META0:![0-9]+]] {
+; CHECK-NEXT: ret i32 1
+;
+; THRESHOLD-0-LABEL: define i32 @func0(
+; THRESHOLD-0-SAME: ) !guid [[META0:![0-9]+]] {
+; THRESHOLD-0-NEXT: ret i32 1
+;
ret i32 1
}
define i32 @func1() !guid !1 {
+; CHECK-LABEL: define i32 @func1(
+; CHECK-SAME: ) !guid [[META1:![0-9]+]] {
+; CHECK-NEXT: ret i32 2
+;
+; THRESHOLD-0-LABEL: define i32 @func1(
+; THRESHOLD-0-SAME: ) !guid [[META1:![0-9]+]] {
+; THRESHOLD-0-NEXT: ret i32 2
+;
ret i32 2
}
@@ -20,7 +45,7 @@ define i32 @function_with_jump_table(i32 %index) {
; CHECK-NEXT: switch i32 [[INDEX]], label [[DEFAULT_SWITCH_CASE_UNREACHABLE:%.*]] [
; CHECK-NEXT: i32 0, label [[CALL_0:%.*]]
; CHECK-NEXT: i32 1, label [[CALL_1:%.*]]
-; CHECK-NEXT: ]
+; CHECK-NEXT: ], !prof [[PROF2:![0-9]+]]
; CHECK: default.switch.case.unreachable:
; CHECK-NEXT: unreachable
; CHECK: call.0:
@@ -37,7 +62,7 @@ define i32 @function_with_jump_table(i32 %index) {
; THRESHOLD-0-SAME: i32 [[INDEX:%.*]]) {
; THRESHOLD-0-NEXT: [[GEP:%.*]] = getelementptr inbounds [2 x ptr], ptr @func_array, i32 0, i32 [[INDEX]]
; THRESHOLD-0-NEXT: [[FUNC_PTR:%.*]] = load ptr, ptr [[GEP]], align 8
-; THRESHOLD-0-NEXT: [[RESULT:%.*]] = call i32 [[FUNC_PTR]]()
+; THRESHOLD-0-NEXT: [[RESULT:%.*]] = call i32 [[FUNC_PTR]](), !prof [[PROF2:![0-9]+]]
; THRESHOLD-0-NEXT: ret i32 [[RESULT]]
;
%gep = getelementptr inbounds [2 x ptr], ptr @func_array, i32 0, i32 %index
@@ -54,7 +79,7 @@ define i32 @basic_block_splitted_twice(i32 %index) {
; CHECK-NEXT: switch i32 [[INDEX]], label [[DEFAULT_SWITCH_CASE_UNREACHABLE:%.*]] [
; CHECK-NEXT: i32 0, label [[CALL_0:%.*]]
; CHECK-NEXT: i32 1, label [[CALL_1:%.*]]
-; CHECK-NEXT: ]
+; CHECK-NEXT: ], !prof [[PROF3:![0-9]+]]
; CHECK: default.switch.case.unreachable:
; CHECK-NEXT: unreachable
; CHECK: call.0:
@@ -70,7 +95,7 @@ define i32 @basic_block_splitted_twice(i32 %index) {
; CHECK-NEXT: switch i32 [[INDEX]], label [[DEFAULT_SWITCH_CASE_UNREACHABLE1:%.*]] [
; CHECK-NEXT: i32 0, label [[CALL_02:%.*]]
; CHECK-NEXT: i32 1, label [[CALL_13:%.*]]
-; CHECK-NEXT: ]
+; CHECK-NEXT: ], !prof [[PROF3]]
; CHECK: default.switch.case.unreachable1:
; CHECK-NEXT: unreachable
; CHECK: call.02:
@@ -106,10 +131,22 @@ define i32 @basic_block_splitted_twice(i32 %index) {
}
define void @void_func0() {
+; CHECK-LABEL: define void @void_func0() {
+; CHECK-NEXT: ret void
+;
+; THRESHOLD-0-LABEL: define void @void_func0() {
+; THRESHOLD-0-NEXT: ret void
+;
ret void
}
define void @void_func1() {
+; CHECK-LABEL: define void @void_func1() {
+; CHECK-NEXT: ret void
+;
+; THRESHOLD-0-LABEL: define void @void_func1() {
+; THRESHOLD-0-NEXT: ret void
+;
ret void
}
@@ -123,7 +160,7 @@ define void @void_function_with_jump_table(i32 %index) {
; CHECK-NEXT: switch i32 [[INDEX]], label [[DEFAULT_SWITCH_CASE_UNREACHABLE:%.*]] [
; CHECK-NEXT: i32 0, label [[CALL_0:%.*]]
; CHECK-NEXT: i32 1, label [[CALL_1:%.*]]
-; CHECK-NEXT: ]
+; CHECK-NEXT: ], !prof [[PROF3]]
; CHECK: default.switch.case.unreachable:
; CHECK-NEXT: unreachable
; CHECK: call.0:
@@ -156,7 +193,7 @@ define void @void_function_with_jump_table_and_call_site_attr(i32 %index) {
; CHECK-NEXT: switch i32 [[INDEX]], label [[DEFAULT_SWITCH_CASE_UNREACHABLE:%.*]] [
; CHECK-NEXT: i32 0, label [[CALL_0:%.*]]
; CHECK-NEXT: i32 1, label [[CALL_1:%.*]]
-; CHECK-NEXT: ]
+; CHECK-NEXT: ], !prof [[PROF3]]
; CHECK: default.switch.case.unreachable:
; CHECK-NEXT: unreachable
; CHECK: call.0:
@@ -183,10 +220,22 @@ define void @void_function_with_jump_table_and_call_site_attr(i32 %index) {
define i32 @func0_addrspace_42() addrspace(42) {
+; CHECK-LABEL: define i32 @func0_addrspace_42() addrspace(42) {
+; CHECK-NEXT: ret i32 1
+;
+; THRESHOLD-0-LABEL: define i32 @func0_addrspace_42() addrspace(42) {
+; THRESHOLD-0-NEXT: ret i32 1
+;
ret i32 1
}
define i32 @func1_addrspace_42() addrspace(42) {
+; CHECK-LABEL: define i32 @func1_addrspace_42() addrspace(42) {
+; CHECK-NEXT: ret i32 2
+;
+; THRESHOLD-0-LABEL: define i32 @func1_addrspace_42() addrspace(42) {
+; THRESHOLD-0-NEXT: ret i32 2
+;
ret i32 2
}
@@ -200,7 +249,7 @@ define i32 @function_with_jump_table_addrspace_42(i32 %index) addrspace(42) {
; CHECK-NEXT: switch i32 [[INDEX]], label [[DEFAULT_SWITCH_CASE_UNREACHABLE:%.*]] [
; CHECK-NEXT: i32 0, label [[CALL_0:%.*]]
; CHECK-NEXT: i32 1, label [[CALL_1:%.*]]
-; CHECK-NEXT: ]
+; CHECK-NEXT: ], !prof [[PROF3]]
; CHECK: default.switch.case.unreachable:
; CHECK-NEXT: unreachable
; CHECK: call.0:
@@ -228,4 +277,18 @@ define i32 @function_with_jump_table_addrspace_42(i32 %index) addrspace(42) {
!0 = !{i64 5678}
!1 = !{i64 5555}
-!2 = !{!"VP", i32 0, i64 25, i64 5678, i64 20, i64 5555, i64 5}
\ No newline at end of file
+!2 = !{!"VP", i32 0, i64 25, i64 5678, i64 20, i64 5555, i64 5}
+;.
+; CHECK: attributes #[[ATTR0]] = { nounwind }
+;.
+; THRESHOLD-0: attributes #[[ATTR0]] = { nounwind }
+;.
+; CHECK: [[META0]] = !{i64 5678}
+; CHECK: [[META1]] = !{i64 5555}
+; CHECK: [[PROF2]] = !{!"branch_weights", i32 0, i32 20, i32 5}
+; CHECK: [[PROF3]] = !{!"unknown"}
+;.
+; THRESHOLD-0: [[META0]] = !{i64 5678}
+; THRESHOLD-0: [[META1]] = !{i64 5555}
+; THRESHOLD-0: [[PROF2]] = !{!"VP", i32 0, i64 25, i64 5678, i64 20, i64 5555, i64 5}
+;.
|
mtrofin
force-pushed
the
users/mtrofin/08-14-_jts_use_common_branch_weigth_downscaling
branch
from
5c51af5 to
dc4a59c
Compare
mtrofin
force-pushed
the
users/mtrofin/08-14-_nfc_pgo_factor_downscaling_of_branch_weights_out_of_instrumentation_into_profiledata_
branch
from
5b1cbec to
8c2429e
Compare
alexander-shaposhnikov
approved these changes
Aug 15, 2025
mtrofin
changed the title
Merge activity
|
mtrofin
force-pushed
the
users/mtrofin/08-14-_nfc_pgo_factor_downscaling_of_branch_weights_out_of_instrumentation_into_profiledata_
branch
2 times, most recently
from
859616d to
5ffa9b3
Compare
Base automatically changed from
users/mtrofin/08-14-_nfc_pgo_factor_downscaling_of_branch_weights_out_of_instrumentation_into_profiledata_
to
main
August 15, 2025 05:44
mtrofin
force-pushed
the
users/mtrofin/08-14-_jts_use_common_branch_weigth_downscaling
branch
from
dc4a59c to
43b197c
Compare
mtrofin
force-pushed
the
users/mtrofin/08-14-_jts_use_common_branch_weigth_downscaling
branch
6 times, most recently
from
0c45985 to
cd7acac
Compare
mtrofin
force-pushed
the
users/mtrofin/08-14-_jts_use_common_branch_weigth_downscaling
branch
from
cd7acac to
91f8130
Compare
mtrofin
deleted the
users/mtrofin/08-14-_jts_use_common_branch_weigth_downscaling
branch
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/134/builds/24454 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This also fixes a bug introduced accidentally in #153651, whereby the
JumpTableToSwitch would convert all the branch weights to 0 except for one. It didn't trip the test becauseupdate_test_checks wasn't run with-check-globals. It is now. This also made noticeable that the direct calls promoted from the indirect call inherited theVPmetadata, which should be dropped as it makes no more sense now.